-
Notifications
You must be signed in to change notification settings - Fork 778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removed use of System.Environment.Exit #129
base: master
Are you sure you want to change the base?
Conversation
Removed System.Environment.Exit(0); and replaced with return; This allows you to run monitor or harvest mode inline (non sacrificial process) without it killing your process.
Replaced System.Environment.Exit(1); with return null; This will prevent the process from being killed if ran inline (in a non sacrificial process).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be an issue ya @0xe7 ?
I'm not 100% because my experience with CS is limited, @CCob may be better to comment on this |
The one for |
Yeah, no issue at all with you changing it to avoid any possible issues. |
Is it still an issue? |
Yes. I'm also having this issue when executing Rubeus in memory. Environment.Exit will also kill the executable Rubeus is running in. |
If someone is willing to modify the PR to throw an ArgumentException for the StringToByteArray function instead of returning null I will happily merge. |
@CCob, has @anthemtotheego given you and other maintainers the right to edit the PR? If this is not the case, it seems that PR #163 has applied the necessary changes. |
Permissions is not the issue, was hoping for the original PR to be updated to save some time that's all. |
Since he's not responding, I think you should take a look at the other PR ( #163 ) that's doing what you asked, don't you? |
Well it certainly won't get looked at when some entitled keyboard warrior demands changes via some passive aggressive commentary. Myself and others involved in this project help maintain it in their own time as and when possible. This PR is far smaller than the other one, which means it's far easier to be confident nothing will break. If you are so keen for a feature that benefits you, why not take this PR, modify the requested line and submit a new one. Then I can review from my phone and merge the commit instead of needing to fire up my PC, review 50 lines of code, check nothing has broken and merge the other PR just for you. |
I couldn't agree with you more! Please accept my apologies if you found me aggressive when that was not at all the intention. It's more about moving forward and wanting the project to have less open PR. The current version of Rubeus suits me perfectly. I wanted to help the author of #163, who has already done what you're asking here! I don't want to steal another PR or create any fight, that's not my war! |
Appreciate the response. I don't think it's about stealing PRs. At the end of the day we all have busy lives, so if a PR has stalled because people are off doing other things I don't think anyone would mind. If the modification is made from a fork of the original PR then all commiters should be included in the commit log anyway. You can also link to this PR if necessary too. I'll see if I have time next week to fully review the other PR. From a quick look on my phone I can see error paths will output more human friendly errors than a stack trace so it certainly has its merits. |
This prevents Rubeus from killing your process when running certain functions inline (or in a non sacrificial process). To reproduce, run current Rubeus through inline-ExecuteAssembly in monitor mode for X seconds. The process is killed and you lose your beacon. If you remove System.Environment.Exit and run the same command, it functions properly and you do not lose your beacon.